-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new example with pynautobot #53
Conversation
9b4c2f1
to
1e6376c
Compare
9f56d1f
to
5288b5f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! A few more comments, plus I saw that you'd marked quite a few of my previous suggestions as resolved without any corresponding code changes, so I've unresolved the ones that appear still relevant.
@@ -197,7 +197,7 @@ def mypy(context, name=NAME, image_ver=IMAGE_VER, local=INVOKE_LOCAL): | |||
""" | |||
# pty is set to true to properly run the docker commands due to the invocation process of docker | |||
# https://docs.pyinvoke.org/en/latest/api/runners.html - Search for pty for more information | |||
exec_cmd = 'find . -name "*.py" | xargs mypy --show-error-codes' | |||
exec_cmd = 'find . -name "*.py" -not -path "*/examples/*" -not -path "*/docs/*" | xargs mypy --show-error-codes' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not run mypy against the example code? Seems like it'd be good practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to pylint, mypy was complaining that we have Duplicate module
between examples.
looks like it's a known limitation python/mypy#4008
LOCAL - Running command find . -name "*.py" | xargs mypy --show-error-codes
examples/example1/models.py: error: Duplicate module named 'models' (also at './examples/example3/models.py') [misc]
Found 1 error in 1 file (checked 29 source files)
Error: Process completed with exit code 123.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to reactivate pylint but based on our discussion i would prefer to keep mypy disabled for now to avoid renaming the files
tasks.py
Outdated
@@ -213,7 +213,7 @@ def pylint(context, name=NAME, image_ver=IMAGE_VER, local=INVOKE_LOCAL): | |||
""" | |||
# pty is set to true to properly run the docker commands due to the invocation process of docker | |||
# https://docs.pyinvoke.org/en/latest/api/runners.html - Search for pty for more information | |||
exec_cmd = 'find . -name "*.py" | xargs pylint' | |||
exec_cmd = 'find . -name "*.py" -not -path "*/examples/*" -not -path "*/docs/*" | xargs pylint' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not run pylint against the example code? Seems like it'd be good practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the list of errors reported by pylint when it's enabled.
It feels like pylint is expecting all python code to be part of the same package and it is not happy because we have some redundant code and library across some examples.
DOCKER - Running command: find . -name "*.py" | xargs pylint container: diffsync-py3.7:1.3.0
************* Module diff
examples/example3/diff.py:11:24: W0612: Unused variable 'child' (unused-variable)
************* Module main
examples/example3/main.py:9:0: C0411: third party import "from local_adapter import LocalAdapter" should be placed before "from diffsync.enum import DiffSyncFlags" (wrong-import-order)
examples/example3/main.py:10:0: C0411: third party import "from nautobot_adapter import NautobotAdapter" should be placed before "from diffsync.enum import DiffSyncFlags" (wrong-import-order)
examples/example3/main.py:11:0: C0411: third party import "from diff import AlphabeticalOrderDiff" should be placed before "from diffsync.enum import DiffSyncFlags" (wrong-import-order)
************* Module local_adapter
examples/example3/local_adapter.py:4:0: E0401: Unable to import 'slugify' (import-error)
examples/example3/local_adapter.py:28:4: W0221: Parameters differ from overridden 'load' method (arguments-differ)
************* Module nautobot_models
examples/example3/nautobot_models.py:2:0: E0401: Unable to import 'pynautobot' (import-error)
examples/example3/nautobot_models.py:86:33: E1101: Instance of 'NautobotCountry' has no 'slug' member (no-member)
examples/example3/nautobot_models.py:5:0: C0411: third party import "from models import Region, Country" should be placed before "from diffsync import DiffSync" (wrong-import-order)
************* Module nautobot_adapter
examples/example3/nautobot_adapter.py:3:0: E0401: Unable to import 'pynautobot' (import-error)
examples/example3/nautobot_adapter.py:79:4: W0222: Signature differs from overridden 'sync_from' method (signature-differs)
examples/example3/nautobot_adapter.py:47:8: W0201: Attribute 'nautobot' defined outside __init__ (attribute-defined-outside-init)
************* Module backend_b
examples/example1/backend_b.py:20:0: E0611: No name 'Site' in module 'models' (no-name-in-module)
examples/example1/backend_b.py:20:0: E0611: No name 'Device' in module 'models' (no-name-in-module)
examples/example1/backend_b.py:20:0: E0611: No name 'Interface' in module 'models' (no-name-in-module)
************* Module backend_c
examples/example1/backend_c.py:20:0: E0611: No name 'Site' in module 'models' (no-name-in-module)
examples/example1/backend_c.py:20:0: E0611: No name 'Device' in module 'models' (no-name-in-module)
examples/example1/backend_c.py:20:0: E0611: No name 'Interface' in module 'models' (no-name-in-module)
************* Module backend_a
examples/example1/backend_a.py:20:0: E0611: No name 'Site' in module 'models' (no-name-in-module)
examples/example1/backend_a.py:20:0: E0611: No name 'Device' in module 'models' (no-name-in-module)
examples/example1/backend_a.py:20:0: E0611: No name 'Interface' in module 'models' (no-name-in-module)
https://github.com/networktocode/diffsync/runs/3861062794
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Does renaming the duplicated models.py
files to example1_models.py
and example3_models.py
respectively help to fix this?
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
This PR adds a new example to show how to use DiffSync to interact with a remote system via its API, like Nautobot.